Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Redux store #927

Open
wants to merge 20 commits into
base: dev
Choose a base branch
from
Open

[WIP] Redux store #927

wants to merge 20 commits into from

Conversation

turnerhayes
Copy link
Collaborator

@turnerhayes turnerhayes commented Oct 30, 2018

Addresses #912

Still todo:

  • Make visualizations use Immutable collections whenever possible
  • Integrate Dev HUD with Redux

@turnerhayes turnerhayes requested a review from knod October 30, 2018 03:01
Copy link
Collaborator

@knod knod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haven't gone through the rest of this yet, but is immutable a peer dependency of Redux? I think that lib was still up for discussion, though I may have forgotten a conversation.

package.json Outdated Show resolved Hide resolved
@wacii
Copy link
Collaborator

wacii commented Oct 30, 2018

Immutable.js isn't a peer dependency. But it's often used with it. And even if you don't use an immutability library, you end up programming in a way that accomplishes much the same thing.

@wacii
Copy link
Collaborator

wacii commented Oct 30, 2018

This is a tremendous amount of progress for a first pass.

@turnerhayes turnerhayes changed the title Redux store [WIP] Redux store Oct 30, 2018
@turnerhayes
Copy link
Collaborator Author

Like @wacii said, immutable is not technically a peer dep; immutability is kind of required for Redux. We can do immutability manually by deep cloning, but it's
a) easy to forget, especially for newcomers/people not familiar with Redux
b) leaves state objects open to (inadvertent) manipulation in code; anyone can push onto an array/set/delete a property, in ways that could potentially break code

It is a different way of coding and can take some getting used to, so we can definitely discuss this more if you're not comfortable with it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants